[1, 2단계 - 체스] 몰리(김지민) 미션 제출합니다. #661
Conversation
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
Co-authored-by: fromitive <[email protected]>
There was a problem hiding this comment.
안녕하세요 현구막!
개인적으로 우테코에 오기 전부터 현구막 블로그를 많이 봤었는데, 이렇게 리뷰를 받게 되어 영광입니다 ㅎㅎ
리뷰 주신 부분을 반영하면서 추가적으로 수정한 부분들이 있어 간단하게 설명 남깁니다..!
(DM 드린 부분은 일단 제외했습니다)
- start 문구 다시 입력 시, 보드판이 초기화가 되지 않아 수정하였습니다.
- Position에서 원시값으로 사용되었던 file과 rank를 enum으로 래핑하였습니다.
- awt 패키지의 Point를 2가지의 용도로의 사용을 분리하였습니다.
- Position 좌표를 나타냄: Point(1,2) → Position(1,2)
- 그대로 Point 사용
- Position으로부터 거리를 나타냄: Point(1,2) → Position(file + 1, rank + 2)
- MoveDistance 객체 생성하여 변경
- Position 좌표를 나타냄: Point(1,2) → Position(1,2)
이번 리뷰도 잘 부탁드립니다 🙏
| public static Point convert(final String position) { | ||
| String[] fileAndRank = position.split(""); | ||
| int file = convertFile(fileAndRank[FILE_INDEX]); | ||
| int rank = Integer.parseInt(fileAndRank[RANK_INDEX]); | ||
|
|
||
| return new Point(file, rank); | ||
| } | ||
|
|
||
| private static int convertFile(final String rawFile) { | ||
| return rawFile.charAt(FIRST_CHARACTER_INDEX) - ASCII_DIFFERENCE_FROM_A; | ||
| } |
There was a problem hiding this comment.
👍 더욱이 좌표가 문자열이 아닌 다른 방식으로 전달된다고 하면 (체스는 그럴 일이 적겠지만..) view 에서 변화가 생겨나야겠죠.
| Piece piece = board.get(source); | ||
| if (piece.canMove(source, target, isFirstMove(source, piece), getBoard())) { | ||
| move(source, target, piece); | ||
| return; | ||
| } | ||
|
|
| import java.util.stream.Collectors; | ||
|
|
||
| public record Piece(PieceType pieceType, Color color) { | ||
| private static final Piece EMPTY_PIECE = new Piece(PieceType.EMPTY, Color.NONE); |
| public enum InitialPiecePosition { | ||
| BLACK_KING(new Piece(PieceType.KING, Color.BLACK), List.of(Position.of(5, 8))), | ||
| WHITE_KING(new Piece(PieceType.KING, Color.WHITE), List.of(Position.of(5, 1))), | ||
|
|
There was a problem hiding this comment.
따라서 이번 리팩토링에서 File과 Rank를 enum으로 감싸면서, Position 자체도 enum으로 구현할 수 있겠는데? 하는 생각이 들었는데요.
하지만, 변경이 될 가능성을 아예 없는가?에 대한 생각에서는 확신할 수 없었어요.
👍 좋은 고민입니다!! 당연히
몰리도 잘 아시겠지만, 결론은 "우리 비즈니스(사업) 특성이 어떤가?" 를 파악하고,
그걸 기반으로 설계를 한다... 입니다.
만약 몰리와 함께하는 팀원(특히 기획자나 PO)들이
"앞으로 저희 체스 게임은 좌표 입력 방식을 더 사용자 친화적으로 바꿀거에요." 라는 이야기를 던져두었다면,
몰리는 그 이야기를 귀기울여두었다가 변경이 쉽도록 현재와 같은 설계를 할 수 있겠죠.
그렇지 않다면 잘 변하지 않으므로 몰리의 아이디어처럼 enum 으로 구현해버릴 수도 있구요.
There was a problem hiding this comment.
사업에 관심을 갖고 귀 기울이는 것은 단순히 애정의 영역을 넘어, 일을 잘하는 것과도 관계가 있을텐데요,
만약 사업의 특성을 잘 모르면 혼자만의 설계에 심취하여 변경될 일이 전무한 엄한 부분을 설계하느라 시간을 허비할 수도 있겠죠.
There was a problem hiding this comment.
아하! 상황에 맞게 적절하게 고려하는 것이 역시 중요한 것 같네요...!
의견 감사합니다 현구막 👍
There was a problem hiding this comment.
이번 리뷰도 잘 부탁드립니다 현구막!🙏
이번 요청에서 수정한 부분은 다음과 같습니다!
PawnFirstMovePolicy
FirstMovePolicy를, PawnFirstMovePolicy 으로 변경하여 Pawn만을 위한 정책으로 수정하였습니다.
➡️ 첫 이동 여부를 Board에서 체크하여 주입하는 것이 아닌, PawnFirstMovePolicy내에서 Position를 받아 판별합니다.
컨트롤러 로직
컨트롤러 로직을 게임의 더 흐름을 잘 파악할 수 있도록 수정하였습니다.
+) 추가 수정한 부분
이번 리뷰 요청 텀이 길었어서, 전체 흐름 추가 작성한 PR Description 링크 남깁니다! #661 (comment)
ObstacleRule
기존 코드에서는 Piece에서,
Map<Position, Piece> pieces로부터 도착 위치와 목표 위치를 비교하여 장애물 목록을 생성합니다.- 가능한 movement에 대해 순회하며 target으로 도착할 수 있는지를 확인합니다.
그런데 요구사항 중 '모든 엔티티를 작게 유지한다.'에 대한 항목이 걸리기도 하였고, 장애물 생성 로직 중 폰 여부에 따라 추가적인 로직이 필요하기 때문에 분리하는 것이 좋을 것 같다는 생각이 들었습니다! 🙃
따라서, ObstacleRule를 생성하여 말 타입 별 공격 조건에 따라 DiagonalCaptureObstacleRule, StraightCaptureObstacleRule를 구현하였습니다!
- StraightCaptureObstacleRule -> 목표 위치가 상대편 말이고 공격하기 위해, 직선으로 이동해야 하는 경우
- DiagonalCaptureObstacleRule -> 목표 위치가 상대편 말이고 공격하기 위해, 대각선으로 이동해야 하는 경우
턴 기능 관련 생각
이번 리뷰를 반영하면서 턴 기능을 도입해보는 것을 생각해보았는데요.
실제 체스와 같다면, 2명의 플레이어가 번갈아 가면서 게임이 진행되기 때문에 턴이 있는 것이 맞다고 생각을 했습니다.
하지만 이번 미션은 콘솔 기반이기도 하고, 사용자가 이동을 안하고 턴 패스를 할 수도 있다고 생각했습니다.
마지막으로 턴이 없어도 게임 진행에 큰 영향이 없는 것 같기 때문에, 결론적으로 턴을 넣지 않아도 괜찮을 것이라고 생각했습니다.
이 부분과 관련해서 어느정도 자율성을 보장하면서 사용자를 신뢰~~(라는 표현이 맞는지 모르겠습니다만)~~해도 될지, 아니면 제한을 걸어두는 것이 좋을지 잘 모르겠습니다…!
객체지향 관련 고민
객체지향 관련
마지막으로 코드리뷰와는 조금 별개의 이야기라 바쁘시다면 스킵하셔도 됩니다..!
망설이긴 했지만, 우테코 선배님(?)이신 현구막의 생각이 궁금한 부분이 있습니다..!
저는 그동안 자바와 서버 개발를 공부하면서도, 객체지향에 대해서 깊게 생각해보지 않았었는데요.
우테코에 오고 미션들을 진행하면서 객체지향이 무엇인지를 느끼고, 객체지향적으로 설계를 해보려고 시도했었습니다. 이전 미션들을 진행하고 사람들과 의견을 나누면서 가장 크게 느낀 것은 정답은 없다였습니다.
그런데 이번 체스미션을 하면서, 최근 몇일동안 현타가 크게 왔었습니다…
현재 반영된 movement나 board에 관련된 주요 로직 설계는 페어의 의견을 많이 수용했는데요.
전체적으로 설계가 잘 안 떠오르기도 했고, 시간적인 여유도 없었기 때문에 페어의 설계를 많이 따라가게 되었던 것 같습니다. 그 과정에서 컴포지트 패턴이라는 복잡한 개념을 설명들으면서 페어 당시에는, 속된 말로 뇌를 빼고 개발하게 되버렸습니다.
서론이 조금 두서가 없었던 것 같은데요 🥲
제가 스킬적으로 설계에 대해 많은 경험이나 고민을 겪어보지 않아 그런 것일수도 있지만, 지금 머리 아프게 고민을 하는 객체지향이 백엔드 개발에 있어서 어떻게 활용될지 필요성을 많이 느끼지 못했었습니다…
현구막은 혹시 이런 고민을 해보신 적이 있는지, 이런 경우에는 어떤 마인드셋을 가지는 것이 좋을지 조언 한번 구하고 싶습니다…!🙏
There was a problem hiding this comment.
안녕하세요 몰리! 리팩터링 잘 해주셨습니다 👍
크게 수정된 부분을 정리하여 말씀해주신 덕분에 쉽게 이해할 수 있었어요. 👍 👍
ObstacleRule
관련해서 코멘트를 1개 남겼는데요, 어느정도 코드 중복을 허용하더라도(재활용을 포기하더라도)
더 쉽고 유치하게(!) 코드를 작성하는 것도 고려해보시면 좋겠어요.
개인적으로 그 편이 유지보수가 훨씬 쉽고 유리했던거 같아요 😄 👍
턴 기능
- 턴이 없어도 게임 진행에 영향이 없다 ✅
- 사용자를 신뢰하여 자율성을 보장한다 ❌
고객은 제품을 의도대로만 사용하지 않습니다 ㅋㅋㅋ
온갖 기상천외한 방법을 다 사용하기 때문에... 제약을 둘 수 있다면 가능하면 강력하게 두세요.
다음 단계를 진행하면서 턴을 고민해보셔도 좋겠네요.
객체지향 관련 고민
저는 반대로 생각하는데요, 정답은 있습니다!!
단지 상황별로 답이 모두 다르고, 상황이 너무 많기 때문에... 없는 것처럼 느껴질 뿐이죠... 🥲
객체지향 설계의 기준이자 중심은 결국 유지보수라고 생각해요.
유지보수 하는 시점에 코드가 어떻게 읽힐지, 어떻게 하면 최대한 적은 코드를 수정할지...
그런 고민을 하다보면 자연스럽게 정답이 눈에 보입니다.
저도 우테코 시절 도대체 객체지향이 뭔지 고민이 정말 많았는데요,
어떤 느낌인지 감도 못잡고 대충 코드의 형상을 보면서
"오... 이쁜데? 이게 객체지향인가보다." 라고 만족했던거 같아요.
웃프게도 우테코가 끝나갈 때쯤이 되어서야 깨달음을 얻었어요.
https://hyeon9mak.github.io/consider-oop
오랜 기간 프로젝트를 운영하며 코드를 고치다가 '아 이런건 너무 불편하다...' 를 겪어보지 못했다면
객체지향 설계, DDD 등의 이론이 와닿지 않을 수 밖에 없는거 같아요.
그래서 크루분들이 유지보수 하는 상황을 상상하시면서 '아 이래서 필요하겠구나'를 느끼실 수 있도록
최대한 먼 미래를 예시로 들면서 코멘트를 남기고 있어요.
가령 view 에 대한 책임이 아닐지 코멘트를 남길 때
domain 보다는 view 의 책임이 아닐까요?
대신 아래처럼 말씀드리고 있어요.
동료 개발자에게 "..." 라는 요구사항이 전달된다면, domain 가 view 패키지 중 어디를 먼저 열어볼까요?
당장의 설계가 좋고 나쁨을 따지기보다, 나중에 어떻게 보일지를 계속 상상하고 고민하다보면
몰리도 몰리만의 확신을 갖고 전체적인 설계를 빠르게 해내실 수 있을거라 생각합니다.
테스트 코드가 살짝 깨지는 것 같은데요! 다음 리뷰 요청에 수정해주시면 될 것 같습니다 😄 👍
개인적으로 3~4 단계를 묶어서 진행하기보단 분리해서 진행하고, 4단계만 따로 집중하길 추천드려요.
다음 단계에서 만나요!
| @Override | ||
| public boolean isSatisfied(final Color color, final Position currentPosition, final boolean existEnemy) { | ||
| if (color == Color.BLACK) { | ||
| return BLACK_PAWN.isPawnFirstMove(currentPosition); | ||
| } | ||
|
|
||
| if (color == Color.WHITE) { | ||
| return WHITE_PAWN.isPawnFirstMove(currentPosition); | ||
| } | ||
|
|
||
| return false; | ||
| } |
| public class DiagonalCaptureObstacleRule extends ObstacleRule { | ||
|
|
||
| @Override | ||
| public List<Position> findObstacle(final Position source, final Position target, | ||
| final Map<Position, Piece> pieces) { | ||
| List<Position> obstacles = getNotEmptyPiecePositions(pieces); | ||
| removeCapturableTargetFromObstacle(source, target, pieces, obstacles); | ||
| removeSourcePosition(source, obstacles); | ||
| addTargetToObstacle(source, target, pieces, obstacles); | ||
| return obstacles; | ||
| } | ||
|
|
There was a problem hiding this comment.
개인적으로 드는 생각은 코드 재활용성을 포기하더라도, 각 기물별로 구현체를 나누면 유지보수에 더 쉽지 않을까? 라는 생각이 드네요.
가령 (그럴 일이 없겠지만) ROOK 의 이동전략을 변경한다고 했을 때
동료 개발자가 바로 StraightCaptureObstacleRule 를 인지하고 열어볼 수 있는가? 를 고민해보면 아닐거라는 생각이 들어요.
오히려 RookObstacleRule 같이 유치하고 단순한 방식이 더 잘 먹힐 수 있겠죠. 🤔
There was a problem hiding this comment.
저는 그동안 재활용성을 높이면, 당연하게 유지보수하기 쉬워질 것이라고 생각했는데 리뷰를 보고 생각해보니, 항상 그렇지는 않는 것 같아요!
유지보수를 고려할 때, 설계자 (인 저) 의 관점에서의 유지보수를 생각하게 되버렸던 것 같습니다...
현구막이 자주 말씀해주시는 동료 개발자 관점에서의 유지보수를 고려하는 의식적인 연습을 해야봐야겠어요!
좋은 리뷰 감사합니다!!🫡
|
|
||
| public Position left() { | ||
| return new Position(file.left(), rank); | ||
| } | ||
|
|
||
| public Position right() { | ||
| return new Position(file.right(), rank); | ||
| } | ||
|
|
||
| public Position up() { | ||
| return new Position(file, rank.up()); | ||
| } | ||
|
|
||
| public Position down() { | ||
| return new Position(file, rank.down()); | ||
| } | ||
|
|
||
| public boolean isMinimumFile() { | ||
| return !file.canLeft(); | ||
| } | ||
|
|
||
| public boolean isMaximumFile() { | ||
| return !file.canRight(); | ||
| } | ||
|
|
||
| public boolean isMinimumRank() { | ||
| return !rank.canDown(); | ||
| } | ||
|
|
||
| public boolean isMaximumRank() { | ||
| return !rank.canUp(); | ||
| } | ||
|
|
안녕하세요 현구막!
리뷰 잘 부탁드립니다 😁🙏
전체 흐름
Board에 move 요청Board에서 source의Piece를 확인하여 움직일 수 있는지 확인3-1.
Piece에서PieceType에 해당하는Movement을 조회3-2.
Movement중Color, 공격 가능 유무, 첫 이동 여부 등을 통해 현재 적용 가능한Movement만 필터링3-3. 필터링된 Movement을 순회하며 도달할 수 있는
Direction이 있는지 확인3-3-1. 이때
PieceType에 해당하는ObstacleRule을 통해 장애물 위치 목록을 전달3-3-2. 도착 위치까지 갈때까지 장애물을 만난다면 즉시 종료 (이동불가)
Piece이동